Repository navigation
Next Python SDK major - #5005
sentrivana wants to merge 414 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5005 +/- ##
===========================================
+ Coverage 70.55% 83.76% +13.21%
===========================================
Files 180 180
Lines 18077 18080 +3
Branches 3008 3009 +1
===========================================
+ Hits 12754 15145 +2391
+ Misses 4432 1943 -2489
- Partials 891 992 +101
|
Codecov Results 📊✅ 55565 passed | ⏭️ 2872 skipped | Total: 58437 | Pass Rate: 95.09% | Execution Time: 152m 59s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 90.07%. Project has 2074 uncovered lines. Coverage diff@@ Coverage Diff @@
## master #PR +/-##
==========================================
- Coverage 90.29% 90.07% -0.22%
==========================================
Files 202 193 -9
Lines 26625 20886 -5739
Branches 9926 7206 -2720
==========================================
+ Hits 24039 18812 -5227
- Misses 2586 2074 -512
- Partials 1515 1237 -278Generated by Codecov Action |
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Anthropic
Documentation 📚
Internal Changes 🔧
Other
🤖 This preview updates automatically when you update the PR. |
Fixes for things that the bots [surfaced](#5005) on the major branch: - some version checks were too late (after patching) - fix TrytondWSGI integration name/`_MIN_VERSIONS` entry mismatch Also, changed the warning of the `DidNotEnable` message from "X not installed" to "X not installed or incompatible".
Originally raised by a bot [here](#5005 (comment)): the `parse_version` function parses version strings as is (e.g. 3.1 becomes `(3, 1)`). We use these parsed version tuples in integrations to compare the installed version against the minimum (defined in `integrations/__init__.py`). The minimum versions are often three-part, e.g. `(3, 1, 0)`. This means that we can mistakenly consider a valid version to be below the minimum, because in pure tuple terms, `(3, 1) < (3, 1, 0)` is true. This can also happen in reverse (package version has three parts, while our min version boundary has two). In this PR, we make the internal version comparison work as expected regardless of mismatches in the length of the version strings/tuples.
Stop setting `mcp.tool.result.content_count` since the length can be determined from the value of the `gen_ai.tool.call.result` attribute. See deprecation in getsentry/sentry-conventions@b06e641.
- added subsections for integrations, api changes, etc. - added a small span streaming migration guide
…init (#7895) Refs PY-2936
#7896) Refs PY-2936 --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>
…ry_init (#7899) Refs PY-2936
…it (#7901) Refs PY-2936
…sentry_init (#7903) Refs PY-2936
…_init (#7904) Refs PY-2936
…nit (#7905) Refs PY-2936
…ion` from sentry_init (#7906) Refs PY-2936
…undant empty `data_collection` from sentry_init (#7907) Refs PY-2936
…d, requests, rq, starlette, starlite, stdlib, strawberry, wsgi): Remove redundant empty `data_collection` from sentry_init (#7908) Refs PY-2936 --------- Co-authored-by: Pablo Deputter <71842639+pabloDeputter@users.noreply.github.com>
### Description Adds the following request/response attributes: `aws.s3.bucket`, `aws.s3.key`, `aws.s3.upload_id`, `aws.s3.copy_source`, `aws.s3.delete`, `aws.s3.part_number`, `http.response.body.size` (number of bytes in payload), `file.size` (represents the total file/object size) Following tests were removed/moved from old `test_s3.py` (old boto3 integration only added s3 streaming responses support): - `test_basic()` removed - basically only tests botocore xml parsing behavior; generic span behavior is already tested in `test_client.py`. - `test_streaming()` / `test_streaming_close()` removed - behavior is already tested in `test_client.py` with mock HTTP server. - `test_span_origin()` removed - overlaps with existing tests. - `test_omit_url_data_if_parsing_fails()` moved to `test_client.py` and simplified since it's generic instrumentation. - `test_breadcrumb()` and `test_url_query_data_collection_breadcrumb()` moved and combined to `test_client.py`, once again since it's generic instrumentation. #### Issues Resolves #7576
) Group the TracingTestClass tracing tests into a class whose setup/teardown restores the `static` and `class_` attributes that `functions_to_trace` patches. Resolves warnings like these https://github.com/getsentry/sentry-python/actions/runs/37602693189/job/112730646420#step:6:3739 Refs PY-2637 Refs #6949
The task factory tests build the Sentry wrapper coroutine but never run it, so it is garbage-collected unawaited. This surfaces as a "coroutine was never awaited" RuntimeWarning attributed to an unrelated later test (e.g. test_span_origin). Close the coroutine once the assertions are done. Fixes PY-2943 Fixes #7923
| "cookies": { "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"] }, | ||
| "url_query_params": { | ||
| "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"], | ||
| }, |
There was a problem hiding this comment.
Migration recipe leaves URL query parameters enabled
The migration recipe claims to roughly match send_default_pii=False, but configures url_query_params in denylist mode. That mode only redacts keys matching built-in or configured terms; other query keys and their values are retained and attached to request data or span URL attributes. Set url_query_params to {"mode": "off"} to preserve the previous opt-out from query-string collection.
Evidence
- The migration recipe configures
url_query_paramswithmode: denylistand terms that do not match ordinary keys such asemailorsearch(MIGRATION_GUIDE.md). _apply_key_value_collection_filteringpreserves values for keys that do not match the sensitive denylist or configured terms (sentry_sdk/data_collection.py).- ASGI request extraction attaches a nonempty filtered query string, and URL attributes include it in
http.query/url.full(sentry_sdk/integrations/_asgi_common.py). - The changelog records that query strings were gated behind
send_default_piiin multiple integrations (CHANGELOG.md).
Identified by Warden · code-review · ZG6-PV7
| if lazy_mode and not threads_enabled: | ||
| from warnings import warn | ||
|
|
||
| warn( | ||
| Warning( | ||
| "IMPORTANT: " | ||
| "We detected the use of uWSGI without thread support. " | ||
| "This might lead to unexpected issues. " | ||
| 'Please run uWSGI with "--enable-threads" for full support.' | ||
| ) | ||
| from sentry_sdk.utils import logger | ||
|
|
||
| logger.warning( | ||
| "IMPORTANT: " | ||
| "We detected the use of uWSGI without thread support. " | ||
| "This might lead to unexpected issues. " | ||
| 'Please run uWSGI with "--enable-threads" for full support.' | ||
| ) | ||
|
|
||
| return False | ||
|
|
||
| elif not lazy_mode and (not threads_enabled or not fork_hooks_on): | ||
| from warnings import warn | ||
|
|
||
| warn( | ||
| Warning( | ||
| "IMPORTANT: " | ||
| "We detected the use of uWSGI in preforking mode without " | ||
| "thread support. This might lead to crashing workers. " | ||
| 'Please run uWSGI with both "--enable-threads" and ' | ||
| '"--py-call-uwsgi-fork-hooks" for full support.' | ||
| ) | ||
| from sentry_sdk.utils import logger | ||
|
|
||
| logger.warning( | ||
| "IMPORTANT: " | ||
| "We detected the use of uWSGI in preforking mode without " | ||
| "thread support. This might lead to crashing workers. " | ||
| 'Please run uWSGI with both "--enable-threads" and ' | ||
| '"--py-call-uwsgi-fork-hooks" for full support.' | ||
| ) |
There was a problem hiding this comment.
uWSGI support warnings are suppressed during fresh client initialization
Both uWSGI misconfiguration messages use logger.warning, which is filtered unless initialization debug mode is active or the current client has debug=True. Client._init_impl() restores the initialization debug flag before calling check_uwsgi_thread_support(), and init() attaches the new client to the scope only after its constructor returns. Consequently, during fresh initialization, the warnings are suppressed even for init(debug=True), hiding guidance for configurations that may cause worker crashes. Consider preserving the always-visible warnings.warn behavior or otherwise allowing these warnings through the filter.
Evidence
check_uwsgi_thread_support()insentry_sdk/_compat.pyuseslogger.warningfor both missing uWSGI thread support and missing prefork fork hooks.configure_logger()attaches_DebugFilterto that logger; the filter passes records only when_client_init_debugis true or the current client'sdebugoption is true.Client._init_impl()restores_client_init_debugin itsfinallyblock before callingcheck_uwsgi_thread_support();_init_implementation._init()attaches the new client to the global scope only after the constructor returns.- Thus, on fresh initialization, even
init(debug=True)reaches the check with the default non-recording client as the current client, so the warnings are filtered. An already-active debug client can change this outcome.
Identified by Warden · code-review, find-bugs · HLH-Y3D
| if getattr(exc, "_handled_by_sentry", False): | ||
| logger.info("DedupeIntegration dropped duplicated error event %s", exc) | ||
| return None | ||
|
|
||
| # we can only weakref non builtin types | ||
| try: | ||
| integration._last_seen.set(weakref.ref(exc)) | ||
| except TypeError: | ||
| integration._last_seen.set(exc) | ||
|
|
||
| return event | ||
|
|
||
| @staticmethod | ||
| def reset_last_seen() -> None: | ||
| integration = sentry_sdk.get_client().get_integration(DedupeIntegration) | ||
| if integration is None: | ||
| return | ||
|
|
||
| integration._last_seen.set(None) | ||
| else: | ||
| with capture_internal_exceptions(): | ||
| exc._handled_by_sentry = True | ||
| return event |
There was a problem hiding this comment.
Dedupe suppresses a repeated exception after before_send drops it
The dedupe processor marks an exception as handled before before_send runs. If before_send drops the event, the marker remains; capturing that same exception instance again causes the processor to drop it as a duplicate. Reset the marker when the event is dropped, or mark the exception only after the event is accepted.
Evidence
DedupeIntegration.processor()setsexc._handled_by_sentry = Truefor an exception hint before returning the event.Client._prepare_event()runs scope event processors before callingbefore_send; when that callback returnsNone, it records the loss but does not clear the exception marker.- A later
capture_exception()of the same instance passes it inhint["exc_info"], so the dedupe processor sees the marker and returnsNone. - The existing dropped-exception test creates a new
ValueErroron each iteration, so it does not cover recapturing the same instance.
Also found at 1 additional location
sentry_sdk/client.py:558-566
Identified by Warden · find-bugs, code-review · FQ4-JUB
| with sentry_sdk.start_span( | ||
| name=function_name, | ||
| parent_span=None, | ||
| attributes={ | ||
| "sentry.op": OP.FUNCTION_GCP, | ||
| "sentry.origin": GcpIntegration.origin, | ||
| "sentry.segment.name.source": SegmentNameSource.COMPONENT, | ||
| "cloud.provider": CLOUD_PROVIDER.GCP, | ||
| "faas.name": function_name, | ||
| **header_attributes, | ||
| **additional_attributes, | ||
| }, | ||
| ): | ||
| try: | ||
| return func(functionhandler, gcp_event, *args, **kwargs) | ||
| except Exception: | ||
| exc_info = sys.exc_info() | ||
| sentry_event, hint = event_from_exception( | ||
| exc_info, | ||
| client_options=client.options, | ||
| mechanism={"type": "gcp", "handled": False}, | ||
| ) | ||
| sentry_sdk.capture_event(sentry_event, hint=hint) |
There was a problem hiding this comment.
GCP flush runs before the invocation span is queued
client.flush() runs in the finally inside the start_span context. With streamed spans, the segment is queued only when that context exits, so this flush does not drain the current invocation's segment. Exiting the context signals the span batcher's asynchronous worker, but does not synchronously send the segment; if the GCP runtime suspends the process, delivery may be delayed until a later invocation or worker flush. Move the flush to a finally that runs after the span context exits.
Evidence
- The GCP wrapper calls
client.flush()in the innerfinally, before thestart_spancontext manager exits. Span.__exit__()calls_end(), which captures the span into the scope;Client.flush()drains batchers, so the current segment is not in the batcher when this flush runs.- After the segment is queued,
SpanBatcher.add()signals its daemon flusher asynchronously. This can send the segment later, but does not guarantee delivery before a GCP invocation is suspended.
Identified by Warden · find-bugs · TXW-7EW
#7930) ### Description Expand boto3 instrumentation with resource identity attributes; new service-extensions (and tests) are added as well (mostly boilerplate, but will be filled in the future). - add `cloud.account.id` and `cloud.resource_id` (https://opentelemetry.io/docs/specs/semconv/resource/cloud/) attributes whenever a request contains a valid AWS ARN; these are extracted using the common helper `_get_aws_arn_attributes()` in `boto3/_utils.py`; the resource's AWS account is used here, not the caller's. It accepts multiple request params. since operations may expose the same resource under different names (the first matching ARN is used). - additionally, following service extensions are registered: DynamoDB, Kinesis, Lambda (file is called `lambda_.py` since it clashes with `lambda` keyword, open for suggestions on renaming it), Secrets Manager, SNS, SQS, and Step Functions. - for SQS, `cloud.account.id` is extracted from `QueueURL` when no ARN is available, e.g. `https://sqs.us-east-2.amazonaws.com/123456789012/MyQueue` (https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-queue-message-identifiers.html) #### Issues Resolves #7929
| "cookies": { "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"] }, | ||
| "url_query_params": { | ||
| "mode": "denylist", "terms": ["forwarded", "-ip", "remote-", "via", "-user"], |
There was a problem hiding this comment.
Cookie denylist in migration sample still sends ordinary cookie values
The example is presented as roughly matching send_default_pii=False, but its cookie denylist only redacts keys matching the listed terms or the built-in sensitive-key list. Ordinary cookie values such as theme and lang are still attached to events. Use "cookies": {"mode": "off"} (or an appropriate allowlist) if the goal is not to collect those cookies.
Evidence
MIGRATION_GUIDE.mdpresents the configuration as roughly matchingsend_default_pii=False, but configures cookies with a denylist of header-oriented terms.data_collection._apply_key_value_collection_filtering()preserves unmatched key/value pairs in denylist mode; modeoffreturns an empty mapping.- Request extractors attach nonempty filtered cookies, and
test_cookie_data_collectionconfirmsthemeandlangsurvive denylist mode whilemode: offomits cookies.
Identified by Warden · find-bugs · JSW-CZZ
| if sentry_sdk.get_current_span() is None: | ||
| return original_method(*args, **kwargs) |
There was a problem hiding this comment.
ensure_integration_enabled fallback calls cache method with wrong args
@ensure_integration_enabled(..., original_method) wraps _instrument_call, whose signature differs from the cache method; if DjangoIntegration is removed after patching, the fallback calls original_method(cache, method_name, original_method, args, kwargs, address, port) and raises TypeError. Decorate sentry_method instead, or fall back with original_method(*args, **kwargs).
Evidence
_instrument_callis decorated with@ensure_integration_enabled(DjangoIntegration, original_method)(line 44) and is invoked as_instrument_call(cache, method_name, original_method, args, kwargs, address, port).ensure_integration_enabledon disable doesreturn original_function(*args, **kwargs)with those same seven arguments.- Bound cache methods like
get(key, default=None, version=None)cannot accept that signature, so cache ops fail after a re-initwithoutDjangoIntegration. - The in-body early return correctly uses
original_method(*args, **kwargs), showing the intended call shape the decorator fallback does not use.
Identified by Warden · find-bugs · B5H-HJS
We're preparing our next major on this branch.
The project is tracked in Linear. If you don't have access, we'll try to tag issues belonging to the project with the
SDK3.0 label on GitHub so that you can follow along.Notable changes
Context
You might have read this announcement about us discontinuing work on a 3.0. This is referring to the work done on the
potel-basebranch, which included two types of changes: a huge refactor of our tracing code on the one hand, and various unrelated changes, improvements and fixes on the other. We're dropping the huge refactor part, and only porting the rest, to a new branch and eventually a new 3.0 release.